Skip to content

Fix #3320: detect dynamic await in pre-Roslyn state machines - #4132

Merged
siegfriedpammer merged 1 commit into
masterfrom
fix/3320-legacy-dynamic-await
Sep 15, 2026
Merged

siegfriedpammer merged 1 commit into
masterfrom
fix/3320-legacy-dynamic-await

Conversation

@siegfriedpammer

Copy link
Copy Markdown
Member

An await on a dynamic value in an assembly built by the C# 5 compiler was
left undecompiled: the state machine struct and its call-site container leaked
into the output as <GetAsync>d__d<T> and <GetAsync>o__SiteContainer6<T>.

The C# 5 compiler loads the awaiter into a fresh local before every dynamic call
site and before the ICriticalNotifyCompletion type test, and the early
non-aggressive inlining refuses to fold those copies. Every await matcher
identifies an await by the identity of its awaiter variable, so each copy hid
the awaiter from the matcher.

Each such copy has one store and one load, and once the dynamic call sites are
collapsed that load sits in the next instruction - the case
ILInlining.InlineOne already handles, including the check that the source is
not overwritten first. Folding is restricted to the IsCompleted / GetResult
call sites and the completion-interface type tests, so dynamic calls in user
code keep their locals; a blanket fold retypes unrelated locals of the
surrounding method (an enum local decompiled as int plus a cast).

Two smaller divergences from the Roslyn shape sat behind that one:

  • the merge block of the AwaitOnCompleted/AwaitUnsafeOnCompleted diamond
    clears doFinallyBodies before its leave, where Roslyn's is a bare leave;
  • the awaiter is restored from its object-typed field with unbox.any rather
    than castclass.

The engine change is +60/-13 in AsyncAwaitDecompiler.cs; the rest of the diff
is the ildasm fixture.

Verification

  • The regression test is the state machine from the issue's assembly with its
    external types stubbed out, so it runs where the legacy compiler does not.
  • Checked against real legacy csc output (/o- and /o+): dynamic await in
    plain code, loops, try/catch, try/finally and using all decompile.
  • Async/await test surface green (127 tests).
  • Both InlineAwaiterCopies call sites were tested for redundancy by removing
    each in turn; either removal leaves the state machine undecompiled. The first
    has to run before detection in AnalyzeStateMachine, the second only becomes
    possible once coalescing has put the call site and its awaiter copy in one
    block.

Known coverage gap

The doFinallyBodies merge-block handling is required for dynamic await inside
try/finally or using - verified against real legacy csc - but the committed
fixture does not exercise that path, so there is no red test behind those lines.
Reviewers may want a second fixture for it.


This pull request was prepared by an AI agent (Claude Opus 5 via Claude Code)
on behalf of the repository owner.

Comment thread ICSharpCode.Decompiler/IL/ControlFlow/AsyncAwaitDecompiler.cs Outdated
@siegfriedpammer
siegfriedpammer force-pushed the fix/3320-legacy-dynamic-await branch from 821923b to 96456e9 Compare September 14, 2026 04:21
The C# 5 compiler loads the awaiter into a fresh local before every
dynamic call site and before the ICriticalNotifyCompletion type test, and
the early non-aggressive inlining refuses to fold those copies. Every
await matcher identifies an await by the identity of its awaiter
variable, so each copy hid the awaiter from the matcher and the whole
await stayed undetected.

Each such copy has one store and one load, and after the dynamic call
sites are collapsed that load sits in the next instruction, which is the
case ILInlining.InlineOne already handles, including the check that the
source is not overwritten first. Folding is restricted to the IsCompleted
and GetResult call sites and the completion-interface type tests, so
dynamic calls in user code keep their locals: a blanket fold retypes
unrelated locals of the surrounding method (an enum local decompiled as
int plus a cast).

Two smaller divergences from the Roslyn shape sat behind that one: the
merge block of the AwaitOnCompleted/AwaitUnsafeOnCompleted diamond
clears doFinallyBodies before its leave, and the awaiter is restored
from its object-typed field with unbox.any rather than castclass.

The ILPretty fixture is the state machine from the issue's assembly with
its external types stubbed out, so the fix stays guarded where the legacy
compiler is unavailable. The Pretty fixture for dynamic await now runs
the pre-Roslyn configurations as well, which is the end-to-end guard on
Windows; its expected output records that the compiler copies an awaited
dynamic value into a local first and emits no debug name for it. The
dead stores an optimized build leaves before a try block containing an
await are not specific to dynamic - a plain await on a Task produces the
same three - so they sit behind an EXPECTED_OUTPUT-only block.

Checked against real legacy csc output (/o- and /o+): dynamic await in
plain code, loops, try/catch, try/finally and using.

Assisted-by: Claude:claude-opus-5[1m]:Claude Code
@siegfriedpammer
siegfriedpammer force-pushed the fix/3320-legacy-dynamic-await branch from 96456e9 to 8519827 Compare September 15, 2026 06:21
@siegfriedpammer
siegfriedpammer merged commit 96acef1 into master Sep 15, 2026
15 checks passed
@siegfriedpammer
siegfriedpammer deleted the fix/3320-legacy-dynamic-await branch September 15, 2026 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant